Skip to content

fix: JSON-RPC ID round-trip preservation for numeric IDs#119

Merged
brandonh-msft merged 7 commits intomainfrom
copilot/fix-113
Aug 1, 2025
Merged

fix: JSON-RPC ID round-trip preservation for numeric IDs#119
brandonh-msft merged 7 commits intomainfrom
copilot/fix-113

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 30, 2025

This PR resolves an issue where JSON-RPC requests with numeric IDs were having their ID values converted to strings in responses, breaking the JSON-RPC specification requirement for ID preservation.

Problem

When sending a JSON-RPC request with a numeric ID (e.g., 123), the A2A server would return responses with the same value converted to a string (e.g., "123"). This violated the JSON-RPC 2.0 specification which requires that response IDs match the exact type and value of the request ID.

Before:

// Request
{ "jsonrpc": "2.0", "id": 123, "method": "tasks/get" }

// Response (incorrect - numeric ID became string)
{ "jsonrpc": "2.0", "id": "123", "result": "..." }

After:

// Request  
{ "jsonrpc": "2.0", "id": 123, "method": "tasks/get" }

// Response (correct - numeric ID preserved)
{ "jsonrpc": "2.0", "id": 123, "result": "..." }

Solution

Introduced a new JsonRpcId struct that preserves the original JSON type (string or number) of JSON-RPC request IDs:

  • Type preservation: Uses an internal object? field to store either string or long values
  • Type checking: Provides IsString, IsNumber, and HasValue properties
  • Type-safe access: Offers AsString() and AsNumber() methods for retrieving typed values
  • Custom serialization: Implements a JsonRpcIdConverter that maintains the original JSON type during serialization/deserialization
  • Implicit conversions: Supports seamless conversion from string, int, and long types

Changes

  • Created JsonRpcId struct with proper equality, conversion operators, and JSON converter
  • Updated JsonRpcRequest and JsonRpcResponse to use JsonRpcId instead of string?
  • Modified JsonRpcRequestConverter to preserve original ID types during deserialization
  • Updated all response factory methods and streaming result classes to work with JsonRpcId
  • Added backward compatibility extensions for A2AExceptionExtensions
  • Updated source generation configuration to include the new type
  • Fixed all existing tests and added comprehensive round-trip preservation tests

Testing

All 240 existing tests pass, plus new tests specifically verify:

  • Numeric IDs (e.g., 123) remain numeric in responses
  • String IDs (e.g., "test-id") remain strings in responses
  • Null IDs are handled correctly
  • Both request/response pairs and error scenarios preserve ID types
  • Round-trip serialization/deserialization maintains original types

This change maintains full backward compatibility while ensuring compliance with the JSON-RPC 2.0 specification.

Fixes #113.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: brandonh-msft <20270743+brandonh-msft@users.noreply.github.com>
Copilot AI changed the title [WIP] If you send a jsonrpc request with ID as a number, the round trip response id will be turned into a string Fix JSON-RPC ID round-trip preservation for numeric IDs Jul 30, 2025
Copilot AI requested a review from brandonh-msft July 30, 2025 19:36
@brandonh-msft brandonh-msft marked this pull request as ready for review July 30, 2025 20:25
@brandonh-msft brandonh-msft changed the title Fix JSON-RPC ID round-trip preservation for numeric IDs fix: JSON-RPC ID round-trip preservation for numeric IDs Jul 30, 2025
@brandonh-msft brandonh-msft enabled auto-merge July 30, 2025 20:25
Copy link
Copy Markdown
Collaborator

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I wish we could just use some library for JSON-RPC related things..

@brandonh-msft brandonh-msft added this pull request to the merge queue Aug 1, 2025
Merged via the queue into main with commit 714ad95 Aug 1, 2025
4 checks passed
@brandonh-msft brandonh-msft deleted the copilot/fix-113 branch August 1, 2025 11:53
throw new ArgumentNullException(nameof(exception));
}

exception.Data[RequestIdKey] = requestId.ToString();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the conversion to string is necessary. Can't we just stick the requestId as is into the data dictionary without conversion? Then, new JsonRpcId(ex.GetRequestId()) won't be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If you send a jsonrpc request with ID as a number, the round trip response id will be turned into a string

4 participants